Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow hogql property queries in replay filtering #26176

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

pauldambra
Copy link
Member

@pauldambra pauldambra commented Nov 13, 2024

we don't allow people to add hogql property filters in replay but we could

this PR

  • allows hogql property queries

## todo

  • convert replay API to not use filters any more behind a flag
  • add a QueryRunner for RecordingsQuery
  • add autocomplete for replay hogql

hogql filtering

2024-11-23 09 03 35

@@ -132,7 +132,7 @@ export const universalFiltersLogic = kea<universalFiltersLogicType>([
newValues.push(newFeatureFlagFilter)
} else {
const propertyType =
item.propertyFilterType ?? taxonomicFilterTypeToPropertyFilterType(taxonomicGroup.type)
item?.propertyFilterType ?? taxonomicFilterTypeToPropertyFilterType(taxonomicGroup.type)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hogql property filters don't have an item so this needs to be slightly safer

@@ -144,6 +147,7 @@ const RecordingsUniversalFilterGroup = (): JSX.Element => {
onRemove={() => removeGroupValue(index)}
onChange={(value) => replaceGroupValue(index, value)}
initiallyOpen={allowInitiallyOpen}
metadataSource={{ kind: NodeKind.RecordingsQuery }}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need something here to correct the autocomplete

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need a query runner in python for the recordings query

@@ -120,6 +120,8 @@ export function convertUniversalFiltersToRecordingsQuery(universalFilters: Recor
actions.push(f)
} else if (isLogEntryPropertyFilter(f)) {
console_log_filters.push(f)
} else if (isHogQLPropertyFilter(f)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this "just works"™

@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week. If you want to permanentely keep it open, use the waiting label.

Copy link
Contributor

github-actions bot commented Nov 23, 2024

Size Change: +110 B (+0.01%)

Total Size: 1.16 MB

ℹ️ View Unchanged
Filename Size Change
frontend/dist/toolbar.js 1.16 MB +110 B (+0.01%)

compressed-size-action

@pauldambra pauldambra marked this pull request as ready for review November 23, 2024 11:05
filter = SessionRecordingsFilter(request=request, team=self.team)
self._maybe_report_recording_list_filters_changed(request, team=self.team)
return list_recordings_response(filter, request, self.get_serializer_context())
use_query_type = (request.GET.get("as_query", "False")).lower() == "true"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's the API client declare which processing version it accepts so we can test the new mechanism with a slow rollout

@pauldambra pauldambra requested a review from a team November 24, 2024 10:23
@pauldambra pauldambra removed the stale label Nov 24, 2024
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@marandaneto
Copy link
Member

@pauldambra is this ready for review?
the PR desc. has a few TODOs and they are not checked, so I am wondering if its not up to date or if this is for later, mind clarifying?

@pauldambra pauldambra removed the request for review from a team November 25, 2024 09:20
@pauldambra
Copy link
Member Author

is this ready for review?

i got over confident at one point and then realised it wasn't done 🙈

will re-tag folk when i've resolved things

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants